Skip to content

fix(purl): filter purl_statuses by SBOM describing CPEs [Backport release/0.4.z] - #2564

Closed
ruromero wants to merge 2 commits into
guacsec:release/0.4.zfrom
ruromero:backport-2523-to-release/0.4.z
Closed

fix(purl): filter purl_statuses by SBOM describing CPEs [Backport release/0.4.z]#2564
ruromero wants to merge 2 commits into
guacsec:release/0.4.zfrom
ruromero:backport-2523-to-release/0.4.z

Conversation

@ruromero

@ruromero ruromero commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Summary

Manual backport of #2523 to release/0.4.z.

The original PR had 8 commits with significant churn (features added then removed within the same PR). The net surviving changes were manually ported to the release/0.4.z code structure, which differs from main in:

  • Entity naming: sbom_package_purl_ref (0.4.z) vs sbom_node_purl_ref (main)
  • Analyze endpoint: raw SQL (0.4.z) vs build_vulnerabilities_query_string helper (main)
  • Vulnerability tests: old API (0.4.z) vs v3 API (main)

Changes

  • purl.rs: Add three-way context_cpe_id filter with generalized CPE matching to the PurlDetails purl_status query, preventing cross-product false positives
  • raw_sql.rs: Remove edition constraint from generalized CPE matching (broadens matches to all CPEs sharing vendor, product, and major version)
  • vulnerability/service/mod.rs: Add context_cpe field to analyze endpoint purl_status JSON
  • dataset.rs: Update ubi8 advisory count assertion (1 → 3) after broadened CPE matching

Test plan

  • cargo fmt — clean
  • cargo clippy -p trustify-module-fundamental — zero warnings
  • Run dataset::ingest test to verify ubi8 advisory count
  • Verify purl details endpoint filters cross-product purl_statuses correctly

Summary by Sourcery

Broaden CPE-based context filtering for purl statuses and expose the associated context CPE in vulnerability analysis responses, aligning behavior with generalized CPE matching and adjusting dataset expectations.

New Features:

  • Include context CPE identifiers in analyze endpoint purl_status JSON responses.

Bug Fixes:

  • Prevent cross-product false positives in purl_status selection by restricting context CPEs to those described by the SBOM for the queried purl.

Enhancements:

  • Extend generalized CPE matching to all CPEs sharing vendor, product, and major version, and apply this logic within the purl details query.

Tests:

  • Update dataset ingestion test expectations for ubi8 advisories to reflect broader CPE matching results.

Manual backport of guacsec#2523 to release/0.4.z. Adapted to the 0.4.z code
structure (sbom_package_purl_ref, raw SQL analyze endpoint).

Changes:
- Add three-way context_cpe_id filter with generalized CPE matching
  to PurlDetails purl_status query (SeaORM)
- Remove edition constraint from generalized CPE matching in raw_sql.rs
- Add context_cpe field to analyze endpoint purl_status JSON
- Update ubi8 advisory count assertion (1 → 3) after broadened matching

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@sourcery-ai

sourcery-ai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Backport of purl context CPE filtering and generalized CPE matching to the 0.4.z branch, tightening purl_status selection to SBOM-relevant CPEs while broadening CPE generalization semantics and exposing context CPE in the analyze endpoint, with tests updated accordingly.

Entity relationship diagram for SBOM-based context CPE filtering and purl_status

erDiagram
    sbom {
        int id
    }

    cpe {
        int id
        string vendor
        string product
        string version
    }

    sbom_describing_cpe {
        int sbom_id
        int cpe_id
    }

    sbom_package_purl_ref {
        int sbom_id
        int qualified_purl_id
    }

    purl_status {
        int id
        int base_purl_id
        int context_cpe_id
    }

    base_purl {
        int id
    }

    qualified_purl {
        int id
    }

    sbom ||--o{ sbom_describing_cpe : has_cpe
    cpe ||--o{ sbom_describing_cpe : described_in

    sbom ||--o{ sbom_package_purl_ref : contains_purl
    qualified_purl ||--o{ sbom_package_purl_ref : referenced_by

    base_purl ||--o{ purl_status : has_status
    cpe ||--o{ purl_status : context_cpe
Loading

Flow diagram for PurlDetails context CPE filtering in purl_status query

flowchart TD
    A[load_package_and_version] --> B[select sbom_ids_for_purl from sbom_package_purl_ref]
    B --> C[select allowed_cpe_ids from sbom_describing_cpe]
    C --> D[union generalized_cpe_ids from cpe via vendor/product/major_version]
    D --> E[build sbom_has_cpes EXISTS subquery]
    E --> F[query purl_status filtered by base_purl_id]
    F --> G{context_cpe_id filter}
    G --> H[context_cpe_id IS NULL]
    G --> I[context_cpe_id IN allowed_cpe_ids]
    G --> J[NOT EXISTS sbom_has_cpes]
Loading

File-Level Changes

Change Details Files
Add SBOM-scoped context CPE filtering to purl_status lookup in PurlDetails to avoid cross-product false positives.
  • Introduce queries to resolve SBOM IDs for the current purl and derive allowed CPE IDs from sbom_describing_cpe with a generalized CPE union.
  • Add an exists-based check to detect SBOMs without describing CPEs.
  • Apply a three-way filter on context_cpe_id (NULL, in allowed CPEs, or SBOM has no CPEs) in the purl_status query while preserving version range handling and distinct_on semantics.
modules/fundamental/src/purl/model/details/purl.rs
Adjust generalized CPE raw SQL to match all CPEs sharing vendor, product, and major version, regardless of edition.
  • Update CONTEXT_CPE_FILTER_SQL CTE generalized_cpes to drop the edition constraint and base matching solely on vendor, product, and split-part major version.
  • Mirror the same generalized_cpes logic change in product_advisory_info_sql to keep advisory queries consistent with the filter semantics.
  • Refresh the inline documentation comment to describe the broadened matching behavior.
modules/fundamental/src/sbom/model/raw_sql.rs
Expose the context CPE associated with each advisory in the analyze endpoint JSON.
  • Extend the jsonb_build_object in the analyze SQL to include a context_cpe field populated from purl_status.context_cpe_id alongside status and id.
modules/fundamental/src/vulnerability/service/mod.rs
Align dataset ingestion test expectations with the broadened CPE matching, which surfaces more ubi8 advisories.
  • Update the ubi8 advisory count assertion from 1 to 3 to reflect additional matches produced by generalized CPE logic.
modules/fundamental/tests/dataset.rs

Possibly linked issues

  • #TC-5171: PR implements the missing three-way context_cpe_id filter in PurlDetails, directly resolving the cross-product false positives.

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 2 issues, and left some high level feedback:

  • The generalized CPE matching logic is now implemented both in the raw SQL constant and in the new SeaQuery builder for allowed_cpe_ids; consider extracting this into a shared helper or at least co-locating the logic so the two stay in sync when behavior changes.
  • The aliases c, sc, and sdc in the generalized CPE query make the join conditions harder to follow; using more descriptive alias names (e.g., concrete_cpe, sbom_cpe, sbom_cpe_link) would improve readability and maintainability of this complex query.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The generalized CPE matching logic is now implemented both in the raw SQL constant and in the new SeaQuery builder for `allowed_cpe_ids`; consider extracting this into a shared helper or at least co-locating the logic so the two stay in sync when behavior changes.
- The aliases `c`, `sc`, and `sdc` in the generalized CPE query make the join conditions harder to follow; using more descriptive alias names (e.g., `concrete_cpe`, `sbom_cpe`, `sbom_cpe_link`) would improve readability and maintainability of this complex query.

## Individual Comments

### Comment 1
<location path="modules/fundamental/src/vulnerability/service/mod.rs" line_range="287-289" />
<code_context>
   jsonb_agg(
     jsonb_build_object(
       'status', status.slug,
-      'id', purl_status.advisory_id
+      'id', purl_status.advisory_id,
+      'context_cpe', purl_status.context_cpe_id
     )
   ) AS advisories
</code_context>
<issue_to_address>
**suggestion:** Consider naming the JSON key to reflect that it carries an ID, not a full CPE object.

The value here is `purl_status.context_cpe_id`, so the key name suggests a full CPE rather than an ID. Consider renaming the key (e.g. to `context_cpe_id`, or whatever matches existing API conventions) to avoid confusing downstream consumers about what this field contains.
</issue_to_address>

### Comment 2
<location path="modules/fundamental/tests/dataset.rs" line_range="97" />
<code_context>
     let ubi_details = ubi_details.unwrap();
     let ubi_advisories = ubi_details.advisories;
-    assert_eq!(ubi_advisories.len(), 1);
+    assert_eq!(ubi_advisories.len(), 3);
     assert!(
         ubi_advisories
</code_context>
<issue_to_address>
**suggestion (testing):** Strengthen this assertion to validate which advisories are present, not just the count.

Simply increasing the expected advisory count to 3 keeps the test brittle and not very descriptive. Since the broader CPE matching is intentional, also assert on which advisories are returned (e.g., IDs, statuses, or a subset) rather than only their number. This will make future failures more informative and clearly document which advisories are expected for ubi8.

Suggested implementation:

```rust
    let ubi_details = ubi_details.unwrap();
    let ubi_advisories = ubi_details.advisories;

    // Assert on the specific advisories we expect for ubi8, not just the count.
    let advisory_ids: std::collections::HashSet<_> = ubi_advisories
        .iter()
        .map(|advisory| advisory.id.as_str())
        .collect();

    let expected_ids: std::collections::HashSet<&'static str> = [
        "RHSA-YYYY:0001",
        "RHSA-YYYY:0002",
        "RHSA-YYYY:0003",
    ]
    .into_iter()
    .collect();

    assert_eq!(
        advisory_ids, expected_ids,
        "unexpected advisories for ubi8: got {:?}, expected {:?}",
        advisory_ids, expected_ids
    );

    assert!(

```

1. Replace `"RHSA-YYYY:0001"`, `"RHSA-YYYY:0002"`, and `"RHSA-YYYY:0003"` with the actual advisory IDs that are expected for the ubi8 test fixture in this test.
2. If the `Advisory` struct does not expose the ID as `advisory.id: String`, adjust the mapping closure `map(|advisory| advisory.id.as_str())` to use the correct field (e.g., `advisory.advisory_id.as_str()` or similar).
3. If you’d like to also assert on statuses or other fields, you can extend the `expected_ids` concept into a struct or tuple set (e.g., `(id, status)`) and map `ubi_advisories` accordingly before comparing the sets.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment on lines 287 to 289
'status', status.slug,
'id', purl_status.advisory_id
'id', purl_status.advisory_id,
'context_cpe', purl_status.context_cpe_id
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: Consider naming the JSON key to reflect that it carries an ID, not a full CPE object.

The value here is purl_status.context_cpe_id, so the key name suggests a full CPE rather than an ID. Consider renaming the key (e.g. to context_cpe_id, or whatever matches existing API conventions) to avoid confusing downstream consumers about what this field contains.

let ubi_details = ubi_details.unwrap();
let ubi_advisories = ubi_details.advisories;
assert_eq!(ubi_advisories.len(), 1);
assert_eq!(ubi_advisories.len(), 3);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Strengthen this assertion to validate which advisories are present, not just the count.

Simply increasing the expected advisory count to 3 keeps the test brittle and not very descriptive. Since the broader CPE matching is intentional, also assert on which advisories are returned (e.g., IDs, statuses, or a subset) rather than only their number. This will make future failures more informative and clearly document which advisories are expected for ubi8.

Suggested implementation:

    let ubi_details = ubi_details.unwrap();
    let ubi_advisories = ubi_details.advisories;

    // Assert on the specific advisories we expect for ubi8, not just the count.
    let advisory_ids: std::collections::HashSet<_> = ubi_advisories
        .iter()
        .map(|advisory| advisory.id.as_str())
        .collect();

    let expected_ids: std::collections::HashSet<&'static str> = [
        "RHSA-YYYY:0001",
        "RHSA-YYYY:0002",
        "RHSA-YYYY:0003",
    ]
    .into_iter()
    .collect();

    assert_eq!(
        advisory_ids, expected_ids,
        "unexpected advisories for ubi8: got {:?}, expected {:?}",
        advisory_ids, expected_ids
    );

    assert!(
  1. Replace "RHSA-YYYY:0001", "RHSA-YYYY:0002", and "RHSA-YYYY:0003" with the actual advisory IDs that are expected for the ubi8 test fixture in this test.
  2. If the Advisory struct does not expose the ID as advisory.id: String, adjust the mapping closure map(|advisory| advisory.id.as_str()) to use the correct field (e.g., advisory.advisory_id.as_str() or similar).
  3. If you’d like to also assert on statuses or other fields, you can extend the expected_ids concept into a struct or tuple set (e.g., (id, status)) and map ubi_advisories accordingly before comparing the sets.

Rust 1.97 introduced clippy::useless_borrows_in_formatting which
flags redundant & in format! macro arguments.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@ruromero
ruromero requested a review from rh-jfuller August 7, 2026 14:11

@rh-jfuller rh-jfuller left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

already in main LGTM

@PhilipCattanach

Copy link
Copy Markdown

@rh-jfuller Could you retrigger the automated tests and assuming they complete successfully merge this PR please?

@rh-jfuller
rh-jfuller enabled auto-merge August 14, 2026 14:30
@rh-jfuller

Copy link
Copy Markdown
Contributor

closing this

@rh-jfuller rh-jfuller closed this Aug 19, 2026
auto-merge was automatically disabled August 19, 2026 10:39

Pull request was closed

@github-project-automation github-project-automation Bot moved this to Done in Trustify Aug 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants